Skip to content

Allow taurus AD private instance access to broker #438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 29, 2025
Merged

Allow taurus AD private instance access to broker #438

merged 2 commits into from
May 29, 2025

Conversation

@DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented May 29, 2025 •
edited
Loading

PR Type

Enhancement

Description

  • Add temporary ingress rule for external IP

  • Permit RabbitMQ access on port 5671/5672

  • Annotate rule for testing purposes

Changes walkthrough 📝

Relevant files
Configuration changes
broker.tf`
Add external IP access to RabbitMQ UI                                       

resources/terraform/auto-drive/broker.tf

  • Insert new ingress block for testing
  • Permit port 15671 from 136.243.147.181/32
  • Add description for external access
    • Additional files
    broker.tf +9/-0     
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @DaMandal0rian DaMandal0rian requested a review from clostao May 29, 2025 16:46
    @github-actions
    Copy link

    github-actions bot commented May 29, 2025 •
    edited by DaMandal0rian
    Loading

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 Security concerns

    Ingress exposure:
    The new rule permits public access to port 5671 from the specified external IP, increasing attack surface. Even if temporary, it can be overlooked and retained in production. Use parameterized CIDR blocks, enforce tighter restrictions, and ensure automatic removal of test-only rules.

    ⚡ Recommended focus areas for review

    Hardcoded IP

    The external IP is hardcoded into the Terraform ingress rule, making updates error-prone and reducing flexibility. Consider parameterizing this address via a variable or data source.

     
    # Allow from specific external IP address temporarily for testing purposes
ingress {
  from_port   = 5671
  to_port     = 5671
  protocol    = "tcp"
  cidr_blocks = ["136.243.147.181/32"]
  description = "Allow RabbitMQ access from external IP"
}
    Temporary rule cleanup

    The temporary ingress rule lacks an automated teardown mechanism, risking prolonged exposure. Implement a lifecycle or automation to revoke this rule after testing completes.

     
    # Allow from specific external IP address temporarily for testing purposes
ingress {
  from_port   = 5671
  to_port     = 5671
  protocol    = "tcp"
  cidr_blocks = ["136.243.147.181/32"]
  description = "Allow RabbitMQ access from external IP"
}

    @github-actions
    Copy link

    github-actions bot commented May 29, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Make external access rule conditional

    Extract this temporary external access rule into its own aws_security_group_rule
    with a conditional count and a variable for the CIDR block. This allows you to
    enable or disable the rule without modifying the shared security group resource
    directly.

    resources/terraform/auto-drive/broker.tf [127-133]

    -ingress {
-  from_port   = 15671
-  to_port     = 15671
-  protocol    = "tcp"
-  cidr_blocks = ["136.243.147.181/32"]
-  description = "Allow RabbitMQ Web UI access from external IP"
+resource "aws_security_group_rule" "rabbitmq_temp_external_access" {
+  count             = var.enable_temp_external_access ? 1 : 0
+  type              = "ingress"
+  from_port         = 15671
+  to_port           = 15671
+  protocol          = "tcp"
+  cidr_blocks       = [var.temp_access_cidr]
+  description       = "Temporary RabbitMQ Web UI access from external IP"
+  security_group_id = aws_security_group.rabbitmq_broker_primary.id
 }
    Suggestion importance[1-10]: 7

    __

    Why: Extracting the temporary ingress rule into a dedicated aws_security_group_rule with conditional count and a variable improves modularity and enables easy toggling without changing the core security group resource.

    		Medium

    clostao
    clostao previously approved these changes May 29, 2025
    View reviewed changes
    @DaMandal0rian DaMandal0rian requested a review from clostao May 29, 2025 17:03
    clostao
    clostao previously approved these changes May 29, 2025
    View reviewed changes
    @DaMandal0rian DaMandal0rian merged commit 4dd73ef into main May 29, 2025
    @DaMandal0rian DaMandal0rian deleted the rabbitmq-access branch May 29, 2025 17:10
    @DaMandal0rian DaMandal0rian mentioned this pull request Jun 2, 2025
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    1 more reviewer

    @clostao clostao clostao approved these changes

    Reviewers whose approvals may not affect merge requirements

    Assignees

    No one assigned

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @DaMandal0rian @clostao